gh-102302 Micro-optimize `inspect.Parameter.__hash__` by Gouvernathor · Pull Request #102303 · python/cpython

Gouvernathor · 2023-02-27T15:56:11Z

Aligns it with eq, enables default properties in subclasses to access hash(self), quickens execution.

No news because imho it's more akin to a fix than to a new feature.

Issue: Make inspect.Parameter.__hash__ use _default instead of default #102302

Aligns it with __eq__, enables default property overrides accessing hash(self), quickens execution.

sobolevn

It is now consistent with __eq__ and __reduce__. LTGM.

AlexWaygood

I'm a little more sceptical about this change; I've posted some questions on the issue.

No news because imho it's more akin to a fix than to a new feature.

Every change that has a user-visible impact (whether it's a bug fix, performance optimisation or feature) needs to have a news entry. If this change doesn't have a user-visible impact, we need to ask why it's worth making this change at all.

AlexWaygood · 2023-03-03T18:59:37Z

Closing; see discussion on the issue for the rationale

sobolevn · 2023-03-03T19:37:10Z

I still think that this is a good idea: using properties here does not make any practical sense.

And direct attribute access is slightly faster.

AlexWaygood · 2023-03-03T19:42:20Z

If somebody can give a benchmark showing a meaningful performance improvement here, I'm happy to reconsider, but the motivation put forward in the issue was not primarily to do with performance. I don't find the rationale in the issue convincing, and I also don't consider an argument to do with "consistency" a strong enough reason to make a change to the stdlib.

This doesn't really fix anything I'd consider a user-visible bug (and if it does, it needs a test/NEWS), so I can't see how it's worth the code churn.

sobolevn · 2023-03-03T22:01:18Z

@AlexWaygood here are my micro-benchmarks.

Old code (hash((selfname, self.kind, self.annotation, self.default))):

(.venv) ~/Desktop/cpython  main ✗                                                    
» pyperf timeit --setup 'import inspect; p = inspect.Parameter("p", inspect.Parameter.POSITIONAL_ONLY, default=4567890, annotation=int)' 'hash(p)'
.....................
Mean +- std dev: 1.87 us +- 0.16 us

New code (hash((self_name, self._kind, self._annotation, self._default))):

(.venv) ~/Desktop/cpython  main ✗                                                    
» pyperf timeit --setup 'import inspect; p = inspect.Parameter("p", inspect.Parameter.POSITIONAL_ONLY, default=4567890, annotation=int)' 'hash(p)'
.....................
Mean +- std dev: 1.05 us +- 0.05 us

I think that the speed-up is quite big for just 4 chars of code :)

AlexWaygood

Please add a short news entry describing the impact this patch will have on users of inspect. Something like this should suffice:

Micro-optimise ``inspect.Parameter.__hash__``, reducing the time it takes to hash an instance by around 40%.

bedevere-bot · 2023-03-04T01:33:28Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Gouvernathor · 2023-03-04T14:42:23Z

So, my overriding the default property will happen to work in the next version, but not be considered a documented feature and could break in future versions without warning ? Just asking to be sure we're on the same page.

AlexWaygood · 2023-03-04T14:45:18Z

So, my overriding the default property will happen to work in the next version, but not be considered a documented feature and could break in future versions without warning ? Just asking to be sure we're on the same page.

Correct -- I'm happy to accept this patch on the basis of the microbenchmark @sobolevn provided, but I still wouldn't recommend doing what you're currently doing in your code base :)

Something along the lines of what I suggested in #102302 (comment) would be safer and less prone to inadvertent breakage.

Gouvernathor · 2023-03-04T14:49:05Z

Judging from this, the :class: should work.

AlexWaygood

Thanks for the patch, and thanks @sobolevn for persuading me to change my mind :)

sobolevn · 2023-03-04T16:42:01Z

🎉

…02303)

* main: (21 commits) pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472) pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455) pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467) pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667) pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664) pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665) pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445) pythonGH-102341: Improve the test function for pow (python#102342) Fix unused classes in a typing test (pythonGH-102437) pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318) pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426) Move around example in to_bytes() to avoid confusion (python#101595) pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421) pythongh-96821: Add config option `--with-strict-overflow` (python#96823) pythongh-101992: update pstlib module documentation (python#102133) pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699) pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417) pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303) pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180) pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896) ...

* main: (37 commits) pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472) pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455) pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467) pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667) pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664) pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665) pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445) pythonGH-102341: Improve the test function for pow (python#102342) Fix unused classes in a typing test (pythonGH-102437) pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318) pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426) Move around example in to_bytes() to avoid confusion (python#101595) pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421) pythongh-96821: Add config option `--with-strict-overflow` (python#96823) pythongh-101992: update pstlib module documentation (python#102133) pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699) pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417) pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303) pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180) pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896) ...

Make Parameter.__hash__ use the actual values r.t properties

604425d

Aligns it with __eq__, enables default property overrides accessing hash(self), quickens execution.

bedevere-bot added the awaiting review label Feb 27, 2023

bedevere-bot mentioned this pull request Feb 27, 2023

Make inspect.Parameter.__hash__ use _default instead of default #102302

Closed

sobolevn added the skip news label Feb 28, 2023

sobolevn approved these changes Feb 28, 2023

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Feb 28, 2023

arhadthedev added the stdlib Standard Library Python modules in the Lib/ directory label Feb 28, 2023

AlexWaygood removed the skip news label Feb 28, 2023

AlexWaygood reviewed Feb 28, 2023

View reviewed changes

AlexWaygood closed this Mar 3, 2023

AlexWaygood reopened this Mar 4, 2023

AlexWaygood requested changes Mar 4, 2023

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Mar 4, 2023

📜🤖 Added by blurb_it.

837bb3f

AlexWaygood changed the title ~~gh-102302 Make Parameter.__hash__ use the actual values r.t properties~~ gh-102302 Micro-optimize inspect.Parameter.__hash__ Mar 4, 2023

AlexWaygood approved these changes Mar 4, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Mar 4, 2023

AlexWaygood merged commit 90801e4 into python:main Mar 4, 2023

bedevere-bot removed the awaiting merge label Mar 4, 2023

Gouvernathor deleted the patch-3 branch March 4, 2023 15:10

hugovk pushed a commit to hugovk/cpython that referenced this pull request Mar 6, 2023

pythongh-102302 Micro-optimize inspect.Parameter.__hash__ (python#1…

2fffbe6

…02303)

Uh oh!

Conversation

Gouvernathor commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Mar 3, 2023

Uh oh!

sobolevn commented Mar 3, 2023

Uh oh!

AlexWaygood commented Mar 3, 2023

Uh oh!

sobolevn commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Mar 4, 2023

Uh oh!

Gouvernathor commented Mar 4, 2023

Uh oh!

AlexWaygood commented Mar 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gouvernathor commented Mar 4, 2023

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Mar 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gouvernathor commented Feb 27, 2023 •

edited

Loading

sobolevn commented Mar 3, 2023 •

edited

Loading

AlexWaygood left a comment •

edited

Loading

AlexWaygood commented Mar 4, 2023 •

edited

Loading